FE: Broker not shown in a list if disk usage is unknown for it #3981
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| import SkewHeader from './SkewHeader/SkewHeader'; | ||
| import * as S from './BrokersList.styled'; | ||
|
|
||
| const NA = 'N/A'; | ||
| const NA = 'N/A' as unknown; |
There was a problem hiding this comment.
if we are going to do type casting can't we do it all of it here ? otherwise it the half of it are someplace.
| it('renders list of all brokers', async () => { | ||
| renderComponent(); | ||
| expect(screen.getByRole('table')).toBeInTheDocument(); | ||
| expect(screen.getAllByRole('row').length).toEqual(3); |
There was a problem hiding this comment.
can we make the .toEqual(3); the number inside to be dynamic based on the mock data ?
|
Kudos, SonarCloud Quality Gate passed! |
|
|
||
| import SkewHeader from './SkewHeader/SkewHeader'; | ||
| import * as S from './BrokersList.styled'; | ||
|
|
||
| const NA = 'N/A'; | ||
| const NA = 'N/A' as unknown as number; |
There was a problem hiding this comment.
I don't like that we cast string to number
There was a problem hiding this comment.
It's because in this component we use N/A as a failsafe for a number types. The other way would be to change autogenerated types to also accept string for size and count props
There was a problem hiding this comment.
we can actually write sanitizer function and link it to useBrokers that way the FE will have control over the type, do it once after we fetch inside the hook useBrokers instead of doing it with useMemo on every render.
What changes did you make? (Give an overview)
Fixed #3615
Is there anything you'd like reviewers to focus on?
How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Check out Contributing and Code of Conduct
A picture of a cute animal (not mandatory but encouraged)